Read SAML parameters from their binding in the .NET Fedlet; run e2e scripts without a shell - #1139
Merged
vharseko merged 1 commit intoSep 29, 2026
Conversation
…cripts without a shell The Fedlet service-provider utility read SAMLRequest, SAMLResponse and SAMLart through HttpRequest's indexer, which also searches cookies and server variables; a SAML binding carries them only in the form of a POST or the query string of a GET, and GetBindingParameter reads them from there. The sample application no longer enables directory browsing, and the SAML e2e spec passes the setup script path to bash as an argument instead of interpolating it into a shell command line.
vharseko
added
security
maximthomas
approved these changes
Sep 29, 2026
maximthomas
left a comment
maximthomas
left a comment
Contributor
There was a problem hiding this comment.
praise: The fix sits at the single point where the Fedlet reads SAML binding parameters.
GetBindingParameter(ServiceProviderUtility.cs:1691) is now the only read path: all 12 formerrequest[...]reads inGetAuthnResponse,GetLogoutRequestandGetLogoutResponsego through it, and no indexer read is left undercsharpsource.- The e2e change was exercised: build-docker at this head ran
saml/saml-test.spec.mjs, whosebeforeAllcallsexecScript, and it passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The .NET Fedlet and e2e cluster of the CodeQL medium triage:
cs/web/ambiguous-client-variable×12,cs/web/directory-browse-enabled×1,js/indirect-command-line-injection×1.The change
ServiceProviderUtility.csreadSAMLRequest,SAMLResponseandSAMLartthroughHttpRequest's indexer (request[name]), which searches QueryString, Form, Cookies and ServerVariables in that order — so a cookie namedSAMLResponsecould stand in for a posted one. A SAML binding carries the message only in the form of a POST or the query string of a GET; the newGetBindingParameter(request, name)reads it from there and nowhere else, and all 12 reads go through it.GetLogoutRequestalready branched onHttpMethod;ProcessSamlResponseandGetLogoutResponsenow do so through the helper.SampleApp/Web.config:directoryBrowseoff. The sample is what people deploy first.e2e/saml/saml-test.spec.mjs:execSync(\bash "${scriptPath}"`)→execFileSync("bash", [scriptPath])` — the path is an argument, not a shell command line.Left as it is, proposed for dismissal
IdentityProviderDiscoveryUtils.cs:171(cs/web/unvalidated-url-redirection): the redirect target is the circle-of-trust reader-service URL from the Fedlet configuration; only theRelayStatequery value comes from the request, and that is the request's own URL.Verification
The Fedlet library compiles with
cscagainst the mono 4.5 framework assemblies (0 errors);node --checkon the spec. No Java module is touched.